-
Notifications
You must be signed in to change notification settings - Fork 1
Fetch with variation option #109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR expands the GraphClient API to support variations filtering when fetching content, transitioning from basic path-based filtering to more flexible content query options. It introduces a dedicated filters.ts
module for building GraphQL query parameters and refactors the existing filtering functions.
- Refactors
fetchContent
andfetchContentType
methods to accept either a string path or options object with path and variation parameters - Creates new internal
filters.ts
module with type-safe GraphQL query building utilities - Updates GraphQL queries to support variation parameters alongside existing where clauses
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
samples/nextjs-template/src/app/json/[...slug]/page.tsx | Updates sample code to use simplified fetchContentType API without explicit filter |
packages/optimizely-cms-sdk/src/index.ts | Removes getFilterFromPath from public exports |
packages/optimizely-cms-sdk/src/graph/index.ts | Refactors GraphClient methods and removes deprecated filter functions |
packages/optimizely-cms-sdk/src/graph/filters.ts | Adds new module with GraphQL query building utilities and TypeScript definitions |
packages/optimizely-cms-sdk/src/graph/error.ts | Updates error types to use new ContentInput interface |
packages/optimizely-cms-sdk/src/graph/createQuery.ts | Updates GraphQL query template to support variation parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added small comments but the rest looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks great!
Not for this PR, but when we add even more general query support, for listing content etc, we should consider renaming functions. For example, fetchContentType()
could be confused with actually loading the content type from the management API, maybe we should call it fetchContentMetadata()
(even though we're just selecting the types from the response now, but we might select more fields in the future). Then maybe make it mandatory to provide which fragments fetch|getContent()
should use and rename the current implementation of this function to routeContent()
where we automatically generate the fragments.
This PR expands the
GraphClient.fetchContent
method to allow path and variation as filter.As a refactor, introduces a new internal
filters.ts
module with type definitions and utility functions (pathFilter
,previewFilter
,variationsFilter
) for building GraphQL query filters, including support for variations and preview parameters.